Add switch level to paths for Show Environment All on C9400s#961
Merged
Conversation
aknopper
changed the title
Contributor
Author
|
@ThomasJRyan Any chance you can look at this? |
ThomasJRyan
requested changes
Oct 16, 2025
Collaborator
ThomasJRyan
left a comment
ThomasJRyan
left a comment
There was a problem hiding this comment.
It's overall good, but due to the schema change this will need to be a revision rather than a modification
Comment on lines
+181
to
+198
| 'switch': { | ||
| Any(): { | ||
| 'power_supply': { | ||
| 'slot': { | ||
| Any(): { | ||
| 'model_no': str, | ||
| 'type': str, | ||
| 'capacity': str, | ||
| 'status': str, | ||
| 'fan_1_state': str, | ||
| 'fan_2_state': str, | ||
| } | ||
| }, | ||
| 'current_configuration_mode': str, | ||
| 'current_operating_state': str, | ||
| 'currently_active': int, | ||
| 'currently_available': int, | ||
| }, |
Collaborator
There was a problem hiding this comment.
A bit of an issue. Due to our strict adherence to backwards compatibility we can't change schemas like this. It has the potential to break older testscripts which we cannot have happen.
We do have a concept of revisions to address this however. It allows us to introduce new revisions of parsers without breaking backwards compatibility.
You can read up on them here https://pubhub.devnetcloud.com/media/pyats-development-guide/docs/writeparser/writeparser.html#revising-a-parser
Contributor
Author
There was a problem hiding this comment.
placed it in /genie/parser/libs/iosxe/rv2/show_platform.py
aknopper
force-pushed
the
showplat9400sw
branch
from
27e9cac to
d688404
Compare
ThomasJRyan
requested changes
Oct 17, 2025
Collaborator
ThomasJRyan
left a comment
ThomasJRyan
left a comment
There was a problem hiding this comment.
Looks good. If I can ask you to provide some unittests I'll be able to approve!
Contributor
Author
|
I added the output files, however I wasn't able to test this locally: Does this module need to be pyats and not ats? I also wonder if putting these revised parsers in the iosxe folder is going to affect other platforms? This is 9400 specific, will genie be able to use the revised one in the iosxe folder if i've defined my device with a model of c9400? |
Collaborator
We actually override the |
Contributor
Author
|
@ThomasJRyan Yes, I have pyats installed: |
Collaborator
|
So the good and bad news is that I can pull your PR into a fresh pyats environment, run the tests, and not get the error you're seeing. Which means it's likely an environment error that can be resolved with a fresh environment install. Though it does seem that the unittest isn't passing which is a concern... My environment Importantly, it seems the tests aren't being picked up Likely just because you haven't moved them into the |
Contributor
Author
|
Okay we should be good now |
ThomasJRyan
approved these changes
Oct 23, 2025
omehrabi
approved these changes
Oct 23, 2025
Contributor
Author
|
@ThomasJRyan It looks like this was removed for the 25.10 release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #958
Switches are included in the "show environment all" output on the CLI, but not being parsed for each switch
Motivation and Context
Now for 9400s with multiple switches, we can parse all power supply and fantray info.
Impact (If any)
New paths, but should support more cases
Screenshots:
Checklist: